[FEAT] 워크스페이스 설정 로직 #193
[FEAT] 워크스페이스 설정 로직 #193
Conversation
| MemberTeamManager memberTeamManager = memberTeamManagerFinder.findByMemberIdAndTeamId(memberId, teamId); | ||
| return MemberTeamPositionGetResponse.from(memberTeamManager.getPosition()); | ||
| return MemberTeamInformGetResponse.from(memberTeamManager.getPosition(), memberTeamManager.getName()); |
There was a problem hiding this comment.
파라미터가 두 개이므로, of로 하면 좋을 거 같아요.
또한 MemberTeamManager 자체를 파라미터로 넘겨도 좋을 거 같아요.
| import com.tiki.server.common.entity.Position; | ||
|
|
||
| public record MemberTeamInformGetResponse( | ||
| Position position, |
There was a problem hiding this comment.
Nonnull, Notnull 같은 널 방지 들어가면 좋을 거 같아요.
| final Principal principal, | ||
| @PathVariable final long teamId | ||
| ) { | ||
| long memberId = Long.parseLong(principal.getName()); | ||
| MemberTeamPositionGetResponse response = memberTeamManagerService.getPosition(memberId, teamId); | ||
| MemberTeamInformGetResponse response = memberTeamManagerService.getMemberTeamInform(memberId, teamId); | ||
| return ResponseEntity.ok().body(success(GET_POSITION.getMessage(), response)); |
There was a problem hiding this comment.
성공메세지랑 api 메소드명이랑 매치가 안되는 거 같아요!
| teamService.updateTeamName(memberId, teamId, request.newTeamName()); | ||
| return ResponseEntity.ok(success(SUCCESS_UPDATE_TEAM_NAME.getMessage())); | ||
| TeamInformGetResponse response = teamService.getTeamInform(teamId); | ||
| return ResponseEntity.ok().body(success(SUCCESS_GET_CATEGORIES.getMessage(), response)); |
There was a problem hiding this comment.
팀 이름을 가져오는 api와 맞는 성공메세지명을 사용하면 좋을 거 같아요.
| final Principal principal, | ||
| @PathVariable final long teamId, | ||
| @RequestBody final UpdateTeamIconRequest request | ||
| @RequestBody final UpdateTeamMemberAndTeamInformRequest request |
There was a problem hiding this comment.
행위가 뒤에 오는 dto명이면 더 좋을 거 같아요.
ex. TeamMemberAndTeamInformUpdateRequest
| return Team.of(request, univ); | ||
| public TeamInformGetResponse getTeamInform(final long teamId) { | ||
| Team team = teamFinder.findById(teamId); | ||
| return TeamInformGetResponse.from(team.getName(), team.getUniv(), team.getIconImageUrl()); |
There was a problem hiding this comment.
파라미터가 적으면 적을 수록 좋은 코드라는 리뷰를 다른 프로젝트에서 받아서 Team 자체를 파라미터로 넘겨도 좋을 거 같아요! 물론 지금도 좋습니다.
| final long teamId, | ||
| final UpdateTeamMemberAndTeamInformServiceRequest request | ||
| ) { | ||
| MemberTeamManager memberTeamManager = checkIsAdmin(memberId, teamId); |
There was a problem hiding this comment.
checkIsAdmin을 통해 ADMIN도 검사하고 멤버팀매니저도 받아오는데 개인적으로 메소드가 두 일을 한다고 생각합니다!
두 로직을 분리해도 좋을 거 같아요!
| @@ -114,8 +118,8 @@ private MemberTeamManager createMemberTeamManager(final Member member, final Tea | |||
| return MemberTeamManager.of(member, team, position); | |||
| } | |||
|
|
|||
| private void deleteIconUrl(final Team team) { | |||
| if (!team.isDefaultImage()) { | |||
| private void updateIconUrlS3(final Team team, final String iconUrl) { | |||
There was a problem hiding this comment.
s3라는 이름이 안 들어가도 괜찮을거 같아요. 만약 추후에 s3를 이용하지 않게된다면 메소드 네이밍을 바꿔야 한다고 생각하기 때문입니다! 만약 들어가는게 좋은거 같다면 updateIconS3Url이 더 나을 거 같아요.
| private void deleteIconUrl(final Team team) { | ||
| if (!team.isDefaultImage()) { | ||
| private void updateIconUrlS3(final Team team, final String iconUrl) { | ||
| if (!team.isDefaultImage() && !team.getImageUrl().equals(iconUrl)) { |
There was a problem hiding this comment.
iconUrl 동등성 검사 로직을 team으로 위임해도 좋을 거 같아요.
| import com.tiki.server.common.entity.University; | ||
|
|
||
| public record TeamInformGetResponse( | ||
| String teamName, |
✨ Related Issue
📝 기능 구현 명세
🐥 추가적인 언급 사항